-
Notifications
You must be signed in to change notification settings - Fork 606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce DescribeReplication #4904
Conversation
522071d
to
51eea7b
Compare
51eea7b
to
ba161eb
Compare
ba161eb
to
5c6d0c2
Compare
⚪ |
⚪
|
⚪
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все коменты минорные в целом ОК
void Handle(NSchemeShard::TEvSchemeShard::TEvDescribeSchemeResult::TPtr& ev, const TActorContext& ctx) { | ||
const auto& record = ev->Get()->GetRecord(); | ||
const auto& desc = record.GetPathDescription(); | ||
|
||
if (record.HasReason()) { | ||
auto issue = NYql::TIssue(record.GetReason()); | ||
Request_->RaiseIssue(issue); | ||
} | ||
|
||
switch (record.GetStatus()) { | ||
case NKikimrScheme::StatusSuccess: | ||
if (desc.GetSelf().GetPathType() != NKikimrSchemeOp::EPathTypeReplication) { | ||
auto issue = NYql::TIssue("Is not a replication"); | ||
Request_->RaiseIssue(issue); | ||
return Reply(Ydb::StatusIds::SCHEME_ERROR, ctx); | ||
} | ||
|
||
ConvertDirectoryEntry(desc.GetSelf(), Result.mutable_self(), true); | ||
return DescribeReplication(desc.GetReplicationDescription().GetControllerId(), | ||
PathIdFromPathId(desc.GetReplicationDescription().GetPathId())); | ||
|
||
case NKikimrScheme::StatusPathDoesNotExist: | ||
case NKikimrScheme::StatusSchemeError: | ||
return Reply(Ydb::StatusIds::SCHEME_ERROR, ctx); | ||
|
||
case NKikimrScheme::StatusAccessDenied: | ||
return Reply(Ydb::StatusIds::UNAUTHORIZED, ctx); | ||
|
||
case NKikimrScheme::StatusNotAvailable: | ||
return Reply(Ydb::StatusIds::UNAVAILABLE, ctx); | ||
|
||
default: { | ||
return Reply(Ydb::StatusIds::GENERIC_ERROR, ctx); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Выглядит так, что мы таскаем эту копипасту из функции в функцию, в данном ревью не критично, но стоит обобщить.
default: | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- default скроет ошибку компилятора при добавлении нового кейса
- отсутствует логирование на неизвестный кейс.
Changelog entry
...
Changelog category
Additional information
...